Skip to content

feat(config): support output.distPath for the artifact build root - #296

Merged
ScriptedAlchemy merged 1 commit into
mainfrom
wave/output-path
Sep 2, 2026
Merged

feat(config): support output.distPath for the artifact build root#296
ScriptedAlchemy merged 1 commit into
mainfrom
wave/output-path

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Summary

  • defineConfig gains an optional output: { distPath?: string } (Rsbuild/Rslib naming; string shorthand only). Precedence: CLI --output > config output.distPath > dist. Existing projects change nothing — projects that had to pass --output artifact because of the AB4706 package-build overlap can now declare it in config.
  • One source of truth: PreparedProject.artifactDistPath (ProjectService resolves config-or-default after loadConfig, merging the directory into outputRoots the same way the eval runs dir already was). Consumers wired: build()/prepack() resolution, inspect (output.distPath on the ready result), source-snapshot exclusion for every command, the dev watcher ignore set (initial + live on re-prepare), and the Workbench host-discovery doctor drift bundleSource that previously hardcoded <root>/dist (feat(workbench): read-only host discovery page over the install doctor (#105 stage 4) #285 wiring) and was already wrong for --output artifact projects.
  • Validation (config values are hashed, portable release-identity inputs, so unlike machine-local Rsbuild config they must stay project-root-contained; absolute paths remain the CLI flag's job): AB4707 malformed block/shape, AB4708 absolute/backslash/.././empty-segment paths, AB4709 reserved first segments (.agent-bundle, .git, node_modules, src). Registered in docs/diagnostics.md.
  • Deliberately deferred: per-asset-type distPath subdirs, output.filename templates, assetPrefix, cleanDistPath — host-pack internals (<target>/skills|mcp|scripts|assets/...) are framework-owned and content-addressed through the artifact manifest, so there are no user-meaningful naming knobs to expose. The config decides WHERE the artifact root lives, never its layout.
  • Docs: framework-mode config reference section; template config comment hints (minimal, cli-tool); changeset (minor).

Test plan

  • Unit: AB4707–AB4709 matrices (shape/path/reserved), acceptance cases, CLI>config>default precedence over real builds, PreparedProject.artifactDistPath incl. malformed fallback + snapshot exclusion of a configured build/artifact tree
  • Integration: custom artifact-out project builds + validates + dev-serves; host-discovery bundleSource reflects configured path (and /dist default); watcher exclusions initial + mid-session config change
  • Local gates on the rebased tree (3c08e6e): typecheck, rslint (0/978), unit + route-unit + projection, integration 764/764 (unit-suite timeouts under load-59 machine conditions re-verified green in isolation, 190/190)

The artifact output was hardcoded to <root>/dist with only the
per-invocation --output flag as an override. defineConfig now accepts
output.distPath (Rsbuild/Rslib naming, string shorthand only): CLI
--output still wins, the default stays dist, and the configured value
flows from one PreparedProject source of truth into build/prepack,
inspect, source-snapshot exclusion, the dev watcher ignore set, and the
Workbench host-discovery doctor drift source that previously pinned
<root>/dist. Config values must be project-root-contained relative POSIX
paths; malformed shapes, root escapes, and reserved namespaces report
AB4707-AB4709. Per-asset distPath subdirs, filename templates,
assetPrefix, and cleanDistPath are deliberately deferred: host-pack
internals are framework-owned and content-addressed, so only WHERE the
artifact root lives is configurable.
@ScriptedAlchemy
ScriptedAlchemy merged commit df52dc7 into main Sep 2, 2026
4 checks passed
@changeset-bot

changeset-bot Bot commented Sep 2, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: dd6b466

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T07:07:09.915572Z dd6b466 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd6b466933

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (segments.some((segment) => segment.length === 0 || segment === '.' || segment === '..')) {
return 'path';
}
return reservedArtifactDistPathSegments.has(segments[0]!) ? 'reserved' : undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject reserved output names case-insensitively

On case-insensitive filesystems, including Windows and typical macOS installations, values such as SRC, .GIT, or Node_Modules refer to the reserved directories but pass this exact-case membership check. A subsequent build publishes by replacing the resolved output directory, so output: { distPath: 'SRC' } can replace and delete the project's actual src tree. Normalize the first segment for platforms with case-insensitive path semantics, or otherwise compare it against the existing filesystem entry before accepting it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a45992e: artifactDistPathIssue now lowercases the first path segment before reserved-name lookup, with regressions for mixed/uppercase src, .git, node_modules, and .agent-bundle.

Comment thread docs/framework-mode.md
Comment on lines +107 to +109
artifact manifest. Unlike machine-local Rsbuild config, the hashed, portable
release-identity config rejects absolute paths; use the per-invocation CLI
flag when an absolute path is required.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not advertise unsupported absolute CLI outputs

When a user follows this new guidance and passes an absolute path outside the project, build() includes that path in ProjectService.outputRoots, whose resolveOutputRoots rejects anything outside the project root and returns AB7002 before the artifact build runs. Therefore the CLI cannot currently serve as the documented escape hatch for absolute output locations; either allow external CLI output roots during preparation or remove this promise (which also appears in AgentBundleOutputConfig).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3c2240169. The framework docs and AgentBundleOutputConfig TSDoc now state that --output only overrides the relative artifact root and remains subject to project-root containment; rejection behavior is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant